Conversation
- Previous state was not locale-dependent
Co-authored-by: Harsh Singh <63696299+harshsingh32@users.noreply.github.com>
Potential Risks and Suggestions:File: js/script.js File: js/script.js File: js/script.js Summary:This PR changes the implementation of a number formatting filter in what appears to be an AngularJS application. The change replaces a custom number formatting algorithm with JavaScript's built-in The new implementation is more concise and leverages browser standards, which is generally a good practice. However, there are potential risks with how decimal values are handled, error handling, and locale-specific formatting differences. Before deploying to production, I recommend:
This change appears to be a refactoring for improved code quality rather than a functional change, but the formatting differences could impact how numbers are displayed to users. |
Description
Previous state hardcoded the thousands separators. I'm not sure which locale uses an apostrophe (') as a thousands separator but for the majority of the world that is not correct. Number.prototype.toLocaleString will format numbers based on user's locale.
Source: https://docs.oracle.com/cd/E19455-01/806-0169/overview-9/index.html
Related Issue
Motivation and Context
Readability
How Has This Been Tested?
Locally
Screenshots (if appropriate):
Types of changes
Checklist: